-
Notifications
You must be signed in to change notification settings - Fork 74
pyproject.toml: PEP 639 compatibility #209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
You may want to add |
|
The default value of |
|
Two 👍, just waiting on a review from someone with merge privileges... |
Signed-off-by: Adam J. Stewart <ajstewart426@gmail.com>
Co-authored-by: Dimitri Papadopoulos Orfanos <3234522+DimitriPapadopoulos@users.noreply.github.com>
Co-authored-by: Dimitri Papadopoulos Orfanos <3234522+DimitriPapadopoulos@users.noreply.github.com>
4df8450 to
d4ca023
Compare
| readme = "README.md" | ||
| license = { file = "LICENSE" } | ||
| license = "MIT-0 AND MIT" | ||
| license-files = ["LICENSE", "src/c/libffi_x86_x64/LICENSE"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds the license for libffi_x86_64. Is this part of the wheel for all platform or just x86_64?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, license-files cannot be configured dynamically, it must be the same for all platforms: https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html#dynamic-metadata. Whether or not the license for libffi should be included or excluded is another question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are also from-source builds that will not bundle libffi at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's distributed with the source dist, isn't it? I would therefore argue the package should reference that licence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I wrote that, I've learned the exact scope of license has been discussed elsewhere. From pypa/pip#13747 (comment):
Whether to include the vendored licenses in the
licensefield was discussed in the PR that added it: pypa/pip#13335Which led to an extensive discussion on DPO about the intent and wording of the PEP: https://discuss.python.org/t/expressing-project-vs-distribution-licenses-post-pep-639/90314. That discussion become too onerous for me to follow, but I believe the conclusion was the
licenseis for the project and there may be future PEPs to cover the distribution licenses? But I could be wrong about that.
See also pypa/pip#13747 (comment).
|
@mattip is this ready to merge now? |
|
Trying to close out old PRs, is this still wanted? |
|
I would like to see this PR merged. |
|
Would anyone with merge privileges still like to see this PR merged? |
|
Truthfully, I don't know enough about the various setuptools versions. Will this break older versions? |
|
This setting requires setuptools 77+. By requiring this in pyproject.toml, pip will ensure that a suitable version is available in the build environment. |
Newer versions support newer specifications, and drop support for deprecated (parts of) specifications. In this case, sometime in the near future table-type license definitions will cause setuptools to raise an exception on build I think setuptools v77 requires Python 3.9+ (I can remember about 3.8) |
|
There's no point trying to keep compatibility with older versions. From the Python Packaging User Guide:
The only constraint is that cffi must support the latest version of setuptools that supports the earliest version of Python supported by cffi. As already stated, setuptools 77.0.3 does support Python 3.9. |
Reverts #199
Fixes #200
PEP 621 is superseded by PEP 639. The previous syntax is deprecated and will be removed in a future version of setuptools.
@Bluefissure @EpicWink @mattip